Improve UX on modal for deleting an access token #19894

Ryuno-Ki · 2022-06-05T17:05:22Z

Associated issue #3589
I am not sure, whether this PR resolves it completely. I could very well
imagine improving the wording here.

Before, both action buttons where coloured on hover. Otherwise they
appeared as ghost buttons. UX tells us, that call to action must not
be displayed as ghost button.

Using red is perceived as warning colour in Western cultures. It was
used for the non-destructive action before. This PR swaps the colour
and turns the cancel button into a filled one, so it is saver to do
nothing then to accidentally delete an access button. We want the
person to do this consciously.

In another iteration the wording here could be improved. See the
associated issue for further details.

Another improvement could be including the name of the token.
Imagine you are interrupted and come back after a pause. Could
you always tell, which token is about to be deleted here?
I don't know, how well this would work with internationalisation, so
I left it open.

Signed-off-by: André Jaenisch andre.jaenisch@posteo.de

Before, both action buttons where coloured on hover. Otherwise they appeared as ghost buttons. UX tells us, that call to action must not be displayed as ghost button. Using red is perceived as warning colour in Western cultures. It was used for the non-destructive action before. This PR swaps the colour and turns the cancel button into a filled one, so it is saver to do nothing then to accidentally delete an access button. We want the person to do this consciously. In another iteration the wording here could be improved. See the associated issue for further details. Signed-off-by: André Jaenisch <andre.jaenisch@posteo.de>

Ryuno-Ki · 2022-06-05T17:14:15Z

Oh, that's interesting 😮
Linting failed, although the formatter did not complain.

Let me fix this.

Linter does not complain anymore. I was expecting the formatter to pick this up but it didn't. Signed-off-by: André Jaenisch <andre.jaenisch@posteo.de>

Ryuno-Ki · 2022-06-05T17:19:53Z

templates/user/settings/applications.tmpl

@@ -23,7 +23,7 @@
 						<div class="content">
 							<strong>{{.Name}}</strong>
 							<div class="activity meta">
-								<i>{{$.i18n.Tr "settings.add_on"}} <span>{{.CreatedUnix.FormatShort}}</span> —  {{svg "octicon-info"}} {{if .HasUsed}}{{$.i18n.Tr "settings.last_used"}} <span {{if .HasRecentActivity}}class="green"{{end}}>{{.UpdatedUnix.FormatShort}}</span>{{else}}{{$.i18n.Tr "settings.no_activity"}}{{end}}</i>


I can back this out if required. But double spaces itched me here. Is it about giving the icon some space?

It's not unusual to sneak in some small code-style into a PR.

Gusted · 2022-06-05T19:59:23Z

templates/user/settings/applications.tmpl

@@ -23,7 +23,7 @@
 						<div class="content">
 							<strong>{{.Name}}</strong>
 							<div class="activity meta">
-								<i>{{$.i18n.Tr "settings.add_on"}} <span>{{.CreatedUnix.FormatShort}}</span> —  {{svg "octicon-info"}} {{if .HasUsed}}{{$.i18n.Tr "settings.last_used"}} <span {{if .HasRecentActivity}}class="green"{{end}}>{{.UpdatedUnix.FormatShort}}</span>{{else}}{{$.i18n.Tr "settings.no_activity"}}{{end}}</i>


It's not unusual to sneak in some small code-style into a PR.

Gusted · 2022-06-05T19:59:45Z

templates/user/settings/applications.tmpl

+	{{/*
+		The wording here is a bit confusing.
+		The action asks for deleting an access token.
+		Confirming is therefore the destructive option and
+		should be deemphasized because it cannot be undone.
+	*/}}


Not sure if we leave comments like this in the code. Haven't seen it before.

I was on the fence. But then looking at the template below where „yes” was in red and no was marked up I thought it warranted an explanation.

Now I don't want to make contributors hunt the code history to find the motivation. And the comment wouldn't get rendered out (I checked). Aside, Go files have comments for functions.

I don't mind if I shall remove it.

Tbh would be better if the comment wouldn't be rendered to the client(if anyone wants to find this motivation, opting to go search it in the code should be de-facto default).

I don't think the template comments are rendered?

They aren't. I double checked.

delvh · 2022-06-05T21:43:53Z

templates/user/settings/applications.tmpl

 			{{.i18n.Tr "modal.no"}}
 		</div>
-		<div class="ui green basic inverted ok button">
+		<div class="ui red basic inverted ok button">
 			<i class="checkmark icon"></i>
 			{{.i18n.Tr "modal.yes"}}


If we are improving UX already, wouldn't it be better to use No -> Cancel and Yes -> Delete instead?

That would trigger a change in the translation. I wrote my thoughts in #3589 (comment)

Therefore, the PR only mentions, it is associated with the issue, i.e. not a complete fix.

How long does it take to complete a translation cycle?
I would fill a follow-up PR to adjust the wording (as I mentioned, I would highlight that the change cannot be undone in this case). Inconsistency in modals are also discussed in #3605

I'm not sure, whether the Template should become a Vue component, for example. Or be made reusable.
I can see arguments pro and con.

"How long does it take to complete a translation cycle?"

PR changes the locale_en-US.ini -> PR gets merged -> 24h (at most) -> text gets translated in crowdin.com -> translation gets approved -> 24h (at most) -> (auto) commit & push the translation.

So, if there are active proof-readers, in (at most) 48h the translations can be updated, and there is still enough time before Gitea 1.17's release.

ps: I think the text could be updated directly without considering the translation cycle. Personally I always update texts if they need to be updated.

Given https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#code-review

Make small pull requests. The smaller, the faster to review and the more likely it will be merged soon.

I will brew a follow-up PR with the wording changes.

This PR is already incredibly small.
This is rather meant for bigger changes, i.e. PRs that change 400+ lines.

* giteaofficial/main: Prevent NPE whilst migrating if there is a team request review (go-gitea#19855) [skip ci] Updated translations via Crowdin Add support for rendering terminal output with colors (go-gitea#19497) Fix viewed images not loading in a PR (go-gitea#19919) Remove out-dated comments (go-gitea#19921) Automatically render wiki TOC (go-gitea#19873) Improve wording on delete access token modal (go-gitea#19909) [skip ci] Updated translations via Crowdin Add breaking email restrictions checker in doctor (go-gitea#19903) Ensure minimum mirror interval is reported on settings page (go-gitea#19895) Improve UX on modal for deleting an access token (go-gitea#19894) update discord invite (go-gitea#19907) Only log non ErrNotExist errors in git.GetNote (go-gitea#19884) [skip ci] Updated translations via Crowdin Update frontend guideline (go-gitea#19901) Make AppDataPath absolute against the AppWorkPath if it is not (go-gitea#19815)

* Improve UX on modal for deleting an access token Before, both action buttons where coloured on hover. Otherwise they appeared as ghost buttons. UX tells us, that call to action must not be displayed as ghost button. Using red is perceived as warning colour in Western cultures. It was used for the non-destructive action before. This PR swaps the colour and turns the cancel button into a filled one, so it is saver to do nothing then to accidentally delete an access button. We want the person to do this consciously. In another iteration the wording here could be improved. See the associated issue for further details. Signed-off-by: André Jaenisch <andre.jaenisch@posteo.de> * Use tabs instead of spaces. Linter does not complain anymore. I was expecting the formatter to pick this up but it didn't. Signed-off-by: André Jaenisch <andre.jaenisch@posteo.de> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: techknowlogick <techknowlogick@gitea.io>

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 5, 2022

Use tabs instead of spaces.

7a877a3

Linter does not complain anymore. I was expecting the formatter to pick this up but it didn't. Signed-off-by: André Jaenisch <andre.jaenisch@posteo.de>

Ryuno-Ki commented Jun 5, 2022

View reviewed changes

Gusted added this to the 1.17.0 milestone Jun 5, 2022

Gusted added type/enhancement An improvement of existing functionality topic/ui-interaction Change the process how users use Gitea instead of the visual appearance labels Jun 5, 2022

Gusted reviewed Jun 5, 2022

View reviewed changes

Gusted approved these changes Jun 5, 2022

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 5, 2022

delvh reviewed Jun 5, 2022

View reviewed changes

delvh approved these changes Jun 6, 2022

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 6, 2022

wxiaoguang and others added 3 commits June 7, 2022 01:25

Merge branch 'main' into ux-3589-delete-access-token-modal

09ff43e

Merge branch 'main' into ux-3589-delete-access-token-modal

2c01ac0

Merge branch 'main' into ux-3589-delete-access-token-modal

0085767

techknowlogick merged commit 81cf006 into go-gitea:main Jun 7, 2022

Ryuno-Ki deleted the ux-3589-delete-access-token-modal branch June 7, 2022 16:31

Ryuno-Ki mentioned this pull request Jun 7, 2022

Improve wording on delete access token modal #19909

Merged

wxiaoguang mentioned this pull request Jun 8, 2022

Remove out-dated comments #19921

Merged

Ryuno-Ki mentioned this pull request Jun 16, 2022

Confirmation prompts: usability, accessibility, and clarity #18014

Closed

4 tasks

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve UX on modal for deleting an access token #19894

Improve UX on modal for deleting an access token #19894

Ryuno-Ki commented Jun 5, 2022

Ryuno-Ki commented Jun 5, 2022

Ryuno-Ki Jun 5, 2022

Gusted Jun 5, 2022

Gusted Jun 5, 2022

Gusted Jun 5, 2022

Ryuno-Ki Jun 5, 2022

Gusted Jun 5, 2022

delvh Jun 5, 2022

Ryuno-Ki Jun 5, 2022

delvh Jun 5, 2022

Ryuno-Ki Jun 5, 2022

wxiaoguang Jun 6, 2022 •

edited

Loading

Ryuno-Ki Jun 6, 2022

delvh Jun 6, 2022

Improve UX on modal for deleting an access token #19894

Improve UX on modal for deleting an access token #19894

Conversation

Ryuno-Ki commented Jun 5, 2022

Ryuno-Ki commented Jun 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxiaoguang Jun 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxiaoguang Jun 6, 2022 •

edited

Loading